Skip to content

.github: add better labeller#33131

Merged
peterbarker merged 1 commit into
ArduPilot:masterfrom
khancyr:labelling
May 21, 2026
Merged

.github: add better labeller#33131
peterbarker merged 1 commit into
ArduPilot:masterfrom
khancyr:labelling

Conversation

@khancyr

@khancyr khancyr commented May 20, 2026

Copy link
Copy Markdown
Contributor

Summary

alternative to #33127. Don't use pull_request_target nor artefact !

Result can be seen here :
khancyr#28

Classification & Testing (check all that apply and add your own)

  • Checked by a human programmer
  • Non-functional change
  • No-binary change
  • Infrastructure change (e.g. unit tests, helper scripts)
  • Automated test(s) verify changes (e.g. unit test, autotest)
  • Tested manually, description below (e.g. SITL)
  • Tested on hardware
  • Logs attached
  • Logs available on request

Description

Comment thread .github/workflows/labels.yml Outdated
Comment on lines +36 to +45
const headSha = context.payload.workflow_run.head_sha;
const result = await github.rest.repos.listPullRequestsAssociatedWithCommit({
owner: context.repo.owner,
repo: context.repo.repo,
commit_sha: headSha,
});
if (result.data.length === 0) {
core.warning('No PR found for commit ' + headSha);
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this works, I like that it's not reliant on outputs from the contributor-controlled workflow, so the user has no control over how the labelling Action gets triggered (e.g. they can't make it trigger on a different PR), just whether or not it does 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@khancyr is it even possible for a commit hash to be looked up in a repo it doesn't exist in / hasn't been merged into?

It seems the endpoint doesn't work if the target owner and repo are specified, which likely explains the test failure.

It does work if we specify the PR author's username and repo name, but we can't do that in an Action being run isolated in the target, because we don't know what those values are without passing them in (in which case we're back to the artifacts approach or similar).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed , we indeed needed to look at OP repo to access it

@ES-Alexander

Copy link
Copy Markdown
Contributor

@khancyr I've raised a PR against your master, to try to test this.

If you could approve the labels trigger workflow run (and cancel the rest if relevant) that would be helpful, so we can see if an external contributor still gets the labels applied correctly :-)

@ES-Alexander ES-Alexander left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!!

@peterbarker peterbarker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -1,22 +1,60 @@
name: ci

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name: ci
name: Labels Apply

@peterbarker peterbarker merged commit d4c759d into ArduPilot:master May 21, 2026
110 checks passed
@khancyr khancyr deleted the labelling branch May 22, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants